Skip to content

fix: close all cross-layer trust gaps from the v0.22.0 deep re-audit (RA-01..RA-24)#72

Merged
CodeWithJuber merged 9 commits into
masterfrom
claude/parallel-solving-3dc0ji
Jul 18, 2026
Merged

fix: close all cross-layer trust gaps from the v0.22.0 deep re-audit (RA-01..RA-24)#72
CodeWithJuber merged 9 commits into
masterfrom
claude/parallel-solving-3dc0ji

Conversation

@CodeWithJuber

Copy link
Copy Markdown
Owner

What & why

The v0.22.0 deep re-audit found that forgekit's trust-sensitive layers fail at their composition boundaries: honest components whose callers discard the result. This PR fixes every code-addressable finding (all 6 P0s, all P1s, and the wording/metadata items), implemented as 7 independently-green workstream commits.

Critical fixes:

  • RA-01forge verify --deep could print PASS/exit 0 when the core verifier reported NOT_CONFIGURED/INCOMPLETE. Deep ok now requires core tests status PASS in addition to lens consensus, and the result carries a four-state status.
  • RA-02 — imported/forged ledger evidence could move confidence. readLog() now recomputes every record's content hash at read time, importState() routes evidence through the full append gate, and rejects are quarantined as sealed audit lines with a surfaced count.

High-severity fixes:

  • RA-03/RA-16/RA-21 — MCP emitters are non-destructive: installed integrations persist in .forge/forge.config.json, sync/add emit the same full managed set (no more oscillation), Continue uses per-server marked files, Codex uses marked refreshable blocks, same-name user entries are never overwritten (--adopt to take over), and new forge integrations remove reverses an add. Stop-hook auto-sync byte-compares the whole file instead of trusting the marker.
  • RA-04/RA-11..13/RA-17/RA-18 — settings merge failures now exit 1 and make install.sh report Install INCOMPLETE; hook paths are shell-quoted (spaces-safe, with quote-normalized dedupe against old installs); invalid profiles abort before any side effect; new forge init --remove-settings reverses the global merge; the installer header tells the truth and announces the merge.
  • RA-05/RA-06git diff/git stash/git cat-file/git archive/git grep are now classified as secret readers; git stash:* moved from allow to ask (no path token to guard against); the secret redactor prints a visible DEGRADED warning on failure and FORGE_GUARD_STRICT=1 makes it blocking.
  • RA-07/RA-10/RA-19/RA-20 — a stale ambient atlas is fully non-authoritative (no predicted tests, no blast-radius enforcement); the completion gate now requires real test evidence for code changes (test file changed or fresh passing forge verify) — a handoff alone no longer satisfies it; doctor reports missing atlas as UNAVAILABLE and failed repairs as failed.

Medium/wording: RA-14 (only minimal/standard profiles; legacy names are deprecated aliases), RA-15 (single config module, corrupt JSON refuses loudly instead of silent overwrite), RA-22/23/24 (package description, README gate and evidence claims match actual behavior).

Each workstream commit independently passed the full gate; the integrated branch finishes at 979 tests / 0 failures, clean Biome, clean tsc, and forge docs check green.

Checklist

  • npm test passes (979 tests, 0 fail on Node 20)
  • npm run check passes (Biome lint + format)
  • New public functions have a test (removeForgeSettings, validateProfile, readForgeConfig/writeForgeConfig, removeIntegration, testRunners, quarantine paths — all covered by new regression tests, +~120 tests)
  • Conventional commit messages (fix:/security:/docs:)
  • CHANGELOG.md updated under ## [Unreleased]
  • No new runtime dependency (zero-dep preserved; YAML/TOML handled with marked-block string handling)
  • Substrate/docs updated (gate decision table, verify states, FORGE_GUARD_STRICT, --remove-settings, integrations remove all documented; forge docs check green)

Risk & rollback

  • Risk level: medium — the completion gate (RA-10) and deep-verify (RA-01) fixes intentionally make previously-passing states fail honestly (NOT VERIFIED, code-without-test-evidence); workflows relying on the old false-passes will see new exit-1s. Fail-open/kill-switch behavior (FORGE_STOPGATE=0, block-once, second-stop-proceeds) is preserved and tested.
  • Rollback plan: revert the whole branch (git revert 65d2a3d..HEAD range) or any single workstream commit — each is self-contained with its own tests.

Extra checks (tick if applicable)

  • npm run typecheck passes
  • Input validated at boundaries; errors handled (no swallowing — that was the point: RA-04/06/15/20 all remove silent-failure paths)
  • Authorization/ownership checked (MCP entry ownership model: managed ∪ adopted only; user entries never clobbered)
  • Logs contain no secrets/PII (secret-shaped test literals assembled via test/_fixtures.js helpers)
  • If AI-assisted: I understand it, verified the package APIs, and it has tests

🤖 Generated with Claude Code

https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6


Generated by Claude Code

claude added 9 commits July 18, 2026 03:37
…data (RA-14, RA-15, RA-22..24)

- repo_config.js is now the single config module: readForgeConfig/writeForgeConfig
  with legacy .forge/config.json migration, unknown-key round-trip, and loud
  refusal (never overwrite) on malformed JSON instead of silent catch{return{}}.
- Profiles reduced to the two that actually differ (minimal, standard); the four
  alias profiles are accepted as deprecated names mapping to standard, with a
  warning at init and rule-load time. validateProfile() exported for pre-flight
  validation.
- package.json description and README gate/evidence claims reworded to match
  actual behavior (advisory by default, FORGE_ENFORCE opt-in; append-time
  evidence validation only).

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
…er; honest CLI states (RA-01, RA-08, RA-09)

- RA-01: verifyDeep ok is now a conjunction — core tests status must be
  PASS AND the lens consensus must not block. A core that never ran a
  verifier (NOT_CONFIGURED/INCOMPLETE) can no longer surface as deep
  ok:true/PASS/exit 0. A four-state `status` field is added (additively)
  to the verifyDeep result, provenance.deep, and the verify metrics
  record (`outcome` vocabulary unchanged).
- RA-08: runTests executes the DETECTED test command via structured
  descriptors from detectStack's new `testRunners` field ({bin, args,
  label}, deduped/sorted; `testCommands` strings unchanged for
  back-compat). Executor whitelist {npm, pnpm, yarn, bun, pytest};
  a pnpm/yarn/bun repo spawns its own package manager, never a
  hardcoded `npm test`. Non-whitelisted detections (go/cargo/mvn/
  gradle/dotnet/bundle/phpunit) and missing binaries (ENOENT) report
  INCOMPLETE with the real label; npx-based runners stay label-only
  and are never executed. FAIL/timeout results carry the real runner
  label.
- RA-09: plain `verify` CLI prints four distinct tests states (pass/
  FAIL with runner, INCOMPLETE with detail, NOT CONFIGURED) and the
  verdict distinguishes BLOCKED (a runner actually failed) from
  NOT VERIFIED (nothing completed). `verify --deep` prints the core
  tests state after the lens table and a four-outcome verdict line.
  Exit code remains 1 for every non-PASS state.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
…valid records (RA-02)

Imported or hand-forged ledger evidence could previously move claim
confidence: importState() appended records via the low-level appendRecord
(bypassing validOutcome/validateRef), and readLog() trusted every line's
`h` without recomputing it, so a forged line with a bogus hash, a known
oracle, and result:"confirm" counted fully in val().

Close every hole at three layers:

- readLog() (the single reader every log goes through) now recomputes each
  line's content hash and drops mismatches; evidence lines must also pass
  validOutcome. Forged lines are invisible to loadState/readEvidence/val/
  blame/dedupe; verify() still names them.
- validOutcome() additionally requires the ref to be a non-empty string
  passing the pure half of validateRef (typed-but-empty refs like `git:`
  rejected; untyped/legacy refs kept — read-path parity with append).
- appendRecord() rechecks the seal, and importState() routes evidence
  through the full appendEvidence gate (validOutcome + git ref resolution
  against the destination repo). Rejected records are quarantined as
  sealed, hash-deduped audit lines under quarantine/<claimId>.log
  (idempotent across re-merges) and counted in the new additive
  `quarantined` field; `forge ledger merge` prints the count. mergeDirs
  reads its source raw on purpose so bad records get quarantined with a
  named reason instead of silently dropped.

Tests: the forged-evidence test now asserts val() is exactly unchanged
(the old <0.75 bound masked real inflation), plus new read-path filtering
and merge-quarantine/idempotence coverage.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
…lth; enforced completion obligations (RA-07, RA-10, RA-19, RA-20)

RA-07 (HIGH): substrateCheck's ambient path kept a stale atlas and derived
impacts, impactedFiles, and predictedTests from it, and enforceDecision could
hard-block on that stale blast radius under FORGE_ENFORCE=1. The stale+ambient
branch now drops the atlas entirely (atlas=null, atlasFresh=false) so every
derivative is empty; predictedTests is additionally gated on atlasFresh, the
blast-radius block requires atlasFresh !== false, and both renderers suppress
the likely-affected-tests section when impact is unavailable. Downstream null
atlas consumers (reusePeek/assembleContext) already fail toward miss/null.

RA-19: doctor reported a missing atlas as ok, which subsystemHealth mapped to
ACTIVE. New "na" result kind (not built/applicable): missing atlas -> health
UNAVAILABLE, stale -> DEGRADED (warn, unchanged), fresh -> ACTIVE; the CLI
prints na neutrally and it never counts toward failed totals.

RA-20: doctor --fix recorded ok:true unless a repair THREW, but mergeSettings
reports failure as a returned {action:"error", reason}. The fix loop now
inspects the returned object and records ok:false with the reason.

RA-10 (HIGH): the completion gate passed any code change as long as a doc or
state artifact moved — `forge handoff` alone satisfied it. gateDecision is now
a real decision table: a code change owes TEST EVIDENCE (a test-class file
moved with it, or a fresh passing `forge verify` provenance stamp written this
session) AND the docs/state continuity leg; test-only sessions pass; config-only
keeps the lighter bar where a handoff alone still satisfies. stopGate reads
.forge/provenance.json parse-guarded (any error -> no evidence), repairReason
leads with the missing leg per row, and all protections are preserved:
FORGE_STOPGATE=0, block-once marker (second stop always proceeds),
stop-hook-active, fail-open on internal errors.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
…aders; visible redactor degradation (RA-05, RA-06)

RA-05: protect-paths.sh only treated git show/log as content readers, so
git diff, cat-file, archive, grep, and stash-show invocations naming a
protected secret path slipped past the Bash secret-read block — and the
settings template pre-allowed two of them. The reader regex now covers
show, log, diff, stash, cat-file, archive, and grep (space or end-of-string
after the subcommand, POSIX-ERE). Plain diff stays allowed in the template
(the guard hook fires on every Bash call and now classifies it), but the
stash rule moves allow -> ask: stash-show with -p dumps stashed secret
content with no path token in the command string, so the permission layer
is the only defense, and stash mutates state.

RA-06: secret-redact.mjs swallowed every failure with exit 0 — a broken
redactor let the original unredacted output pass through with no warning.
Failures now print a visible "secret redaction DEGRADED" line on stderr;
FORGE_GUARD_STRICT=1 turns the degradation into a block (exit 2, mirrored
in the shell launcher's node-missing path). Default stays advisory.
Documented in docs/GUIDE.md env table.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
… profile; reversible settings uninstall (RA-04, RA-11..13, RA-17, RA-18)

RA-04: a failed/refused settings merge now exits 1 with the reason on
stderr — in `forge init --settings-only` (previously a silent exit 0) and
in full `forge init` (previously a console.log with exit 0). install.sh
captures the merge exit status (output captured before the sed pipe, so
sed can no longer mask it), warns on stderr that hooks/guards are NOT
wired, and prints "Install INCOMPLETE" + exits 1 instead of "Done.".

RA-12: resolveManagedPaths now emits single-quoted absolute hook and
statusline paths (`bash '<base>/guards/x.sh' args`, `'\''` escaping) so
an install prefix containing spaces still runs. guardKey dedup is
quote-normalized (and still normalizes legacy `~/.forge/` prefixes), so
old unquoted installed entries match the new quoted template — no
duplicate hooks on re-merge; a matching old unquoted spelling of the
same resolved command is healed in place to the quoted form.

RA-13: `init()` calls the new `validateProfile()` at the top and aborts
with `{profile:{error}, aborted:true}` BEFORE any side effect — an
invalid --profile no longer writes .forge/, AGENTS.md, .gitattributes,
or settings before failing.

RA-17: new `removeForgeSettings()` (`forge init --remove-settings`)
reverses the merge using the settings template as the authoritative
shape: template-keyed hooks, verbatim template permission strings (same
list only), the template statusline and $schema (quote-normalized), and
the _forge marker — user-owned entries survive byte-for-byte, empty
containers are pruned, timestamped backup + atomic write, corrupt file
refused, missing file noop. install.sh --uninstall runs it fail-soft
(manual instruction when node is missing) before removing symlinks.

RA-18: install.sh's header/--help no longer claims it "never edits
settings.json" — it now states the marker-guarded global merge, the new
--no-settings skip flag, and both reversal paths.

RA-11: merge-by-default is kept but announced: `forge init` prints a
"GLOBAL — affects all repos" consent line (with --no-settings /
--remove-settings pointers) on merged AND unchanged runs; install.sh
prints an equivalent line before invoking; README quick-start documents
both flags. FORGE_SETTINGS_PATH (internal, test-only) lets tests target
a throwaway settings file through the CLI.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
Consolidated [Unreleased] entries for the verification, ledger, guards,
substrate/doctor/gate, and init/installer workstreams.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
…plicit adoption; auto-sync full-body compare (RA-03, RA-16, RA-21)

RA-03: `sync` and `integrations add` previously emitted DIFFERENT server sets
(full registry vs the one added server), so sync → add context7 → sync
oscillated — each command clobbered the other's Continue file and never
refreshed stale Codex entries. Installed integrations are now RECORDED in
.forge/forge.config.json (mcp.integrations, via WS7's writeForgeConfig, which
round-trips unknown keys), and both commands emit the same full managed set:
source/mcp.json registry ∪ recorded integrations. A corrupt config emits
registry-only with a warning — never treated as "nothing installed".

- Continue: one forge-marked YAML file per managed server
  (.continue/mcpServers/forge-<name>.yaml) instead of a rebuilt-from-scratch
  combined file. The legacy forge-mcp.yaml is migrated away only when it is
  provably forge's (forge:sync marker, or the old emitter's exact
  `name: Forge MCP` + `schema: v1` fingerprint); anything else stays.
- Codex TOML: each managed server lives in a `# forge:managed:<name>
  begin/end` block that is byte-compared and refreshed on drift. Unmarked
  `[mcp_servers.<name>]` blocks are claimed only when owned (adopted /
  registry) or byte-identical to the old forge emission; otherwise they are
  preserved and reported with an adoption hint.

RA-21: JSON emitters now take an ownership set (registry names implicit ∪
mcp.adopted). Absent entries are written; owned entries refresh on drift; a
same-name entry the user configured is NEVER overwritten — it is reported
with `adopt with: forge integrations add <name> --adopt`. A fresh add that
forge itself creates is auto-adopted (forge owns what it wrote, so catalog
updates keep refreshing it); claiming a pre-existing divergent entry requires
the explicit --adopt.

New `forge integrations remove <name>` reverses an add: clears the record,
deletes the per-server Continue file, the forge-marked Codex block, and JSON
entries that are adopted or byte-identical to forge's spec (a user's own
divergent entry stays). Second remove is a no-op. On a corrupt config both
add and remove refuse instead of writing unrecorded state.

RA-16: autoSyncIfDrifted trusted the embedded forge:sync marker hash, so a
hand-edited AGENTS.md body with an intact marker never resynced. It now
byte-compares the file against the exact content sync would write, built via
the new shared managedContent() helper that writeManaged also writes through,
so the two paths cannot diverge again.

Tests: oscillation matrix across all eight targets, Codex marked-refresh /
user-owned preservation / legacy-block migration, JSON adoption flow,
Continue migration, remove + idempotence, corrupt-config fallback, and the
RA-16 marker-intact drift repair.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants